Conversation
This comment has been minimized.
This comment has been minimized.
…tHibernateIdentity()
creation of synthetic ID properties for entities that lack explicit identifier definitions
…urn types, cross-property arithmetic Bug 2: HibernateQueryExecutor.singleResult() now catches both org.hibernate.NonUniqueResultException and jakarta.persistence.NonUniqueResultException (H7 throws the JPA variant; the original catch missed it) and returns the first result instead of propagating. Bug 4: HqlQueryContext.aggregateTargetClass() now returns precise types per function: count() → Long, avg() → Double, sum/min/max() → Number. Previously all aggregates were bound to Long, causing QueryTypeMismatchException in H7's strict SQM type checking. Bug 5: Cross-property arithmetic in where-DSL (e.g. pageCount > price * 10) was silently dropped — the RHS property reference was coerced to a literal. Fixed via: - PropertyReference: Groovy wrapper returned by propertyMissing for numeric properties; *, +, -, / operators produce a PropertyArithmetic value object - PropertyArithmetic: value type carrying (propertyName, Operator, operand) - HibernateDetachedCriteria: H7-only DetachedCriteria subclass that overrides propertyMissing to return PropertyReference for numeric properties, and newInstance() to preserve the subtype through cloning - HibernateGormStaticApi: overrides where/whereLazy/whereAny to use HibernateDetachedCriteria as the closure delegate - PredicateGenerator: resolveNumericExpression() detects PropertyArithmetic and builds cb.prod/sum/diff/quot(path, operand) instead of a literal H5 and MongoDB are unaffected — all new types are confined to grails-data-hibernate7. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ve on managed entities H7 enforces strict collection identity during flush. GORM's addTo* and save() flow had two failure modes: 1. When an entity is already managed in the current Hibernate session, calling session.merge() causes H7 to create a second PersistentCollection for the same role+key alongside the one already tracked in the session cache -> 'Found two representations of same collection'. Fix (HibernateGormInstanceApi.performMerge): check session.contains(target) before merging. If the entity is already managed, skip merge entirely; dirty-checking and cascade will handle children on flush. 2. When addTo* is called on a managed entity, GormEntity.addTo uses direct field access (reflector.getProperty) which bypasses H7's bytecode-enhanced interceptor, sees null, and creates a plain ArrayList on the field. H7's session cache already tracks a PersistentBag/Set for that role -> two representations on the next save. Fix (HibernateEntity.addTo): override addTo in the H7 trait; for managed entities (id != null), trigger the H7 interceptor via InvokerHelper.getProperty to obtain the live PersistentCollection before delegating to GormEntity.super.addTo. Fix (HibernateEntityTransformation): re-target the concrete addToXxx generated methods so their internal addTo call dispatches through HibernateEntity.addTo rather than being hard-wired to GormEntity.addTo. Fix (HibernateGormInstanceApi.reconcileCollections): detect stale PersistentCollections (session != current session) and replace them with plain collections before merge, covering any edge cases where the H7 interceptor path is not taken. Adds AddToManagedEntitySpec with 4 tests covering: - addTo on an already-persisted entity - multiple addTo on a fresh transient entity - modify child + save twice - removeFrom + save Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…and proper applicationClass - Replace executeQuery(plainString) and executeUpdate(plainString) calls with the (String, Map) overloads (empty map for parameterless queries). HibernateGormStaticApi intentionally rejects plain String in the no-arg overload to prevent HQL injection; parameterless static queries must use the Map overload. - Add applicationClass = Application to @Integration so the spec shares the same application context and transaction manager as the other specs in this module, preventing test-data bleed between specs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # NOTICE # dependencies.gradle # gradle/publish-root-config.gradle
…ibernate7 # Conflicts: # .idea/codeStyles/Project.xml # NOTICE # build-logic/plugins/build.gradle # dependencies.gradle # gradle/publish-root-config.gradle
| import org.apache.tools.ant.taskdefs.condition.Os | ||
|
|
||
| ext { | ||
| if (file('local.properties').exists()) { |
There was a problem hiding this comment.
Is this necessary? What is this being used for?
There was a problem hiding this comment.
local overrides as an option for command line parameters.
There was a problem hiding this comment.
We should move this to our SharedProperties plugin then so it's consistently applied.
There was a problem hiding this comment.
Actually, are you aware that any gradle property can be overridden by environment variable? i.e. 'foo' , set ORG_GRADLE_foo and it will be set that way.
build.gradle
Outdated
| cacheChangingModulesFor(cacheHours, 'hours') | ||
| } | ||
| } | ||
| pluginManager.withPlugin('jacoco') { |
There was a problem hiding this comment.
We should be configuring this in build-logic
There was a problem hiding this comment.
I agree, that is a leftover from working in the module
|
It's my understanding that Walter is working on the spring orm split like we did on the hibernate 5 code. Once that's done, I'll make another pass at the build/license logic then finally review the hibernate code. |
Following the pattern used in Hibernate 5, I have moved the vendored Spring Framework ORM Hibernate 7 integration classes from the core module to a dedicated spring-orm module:
- Created Module: grails-data-hibernate7-spring-orm (located in grails-data-hibernate7/spring-orm).
- Moved Classes: All classes in org.grails.orm.hibernate.support.hibernate7 (including HibernateTransactionManager, HibernateTemplate, LocalSessionFactoryBean, etc.) are now in the new module.
- Updated Dependencies:
- Added the new module to settings.gradle.
- Updated grails-data-hibernate7-core, grails-plugin, boot-plugin, and dbmigration to depend on the new spring-orm module.
- Updated gradle/publish-root-config.gradle to ensure the new module is included in the publishing process.
|
Can we merge without using a github PR into this branch so we can review in one spot? @borinquenkid |
- HibernateDetachedCriteria.isNumericPropertyType: box primitive types before the Number.isAssignableFrom check so that domains declaring numeric properties as primitives (int/long/double/float/short/byte) work correctly in where-DSL arithmetic expressions. Method is protected to allow subclass overrides. - ToManyEntityMultiTenantFilterBinder.bind: add null guard for getHibernateAssociatedEntity() return value to prevent NullPointerException on partially-resolved associations. - grails-data-hibernate7/README.md: add grails-data-hibernate7-spring-orm to the Module Structure table. - Tests: new HibernateDetachedCriteriaSpec covering boxed and all 6 primitive numeric types, non-numeric delegation, and unknown property. Added null-associated-entity test to ToManyEntityMultiTenantFilterBinderSpec. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # grails-data-mongodb/core/src/test/groovy/org/apache/grails/data/mongo/core/GrailsDataMongoTckManager.groovy # grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BeforeUpdatePropertyPersistenceSpec.groovy # grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy # grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/NestedCriteriaWithNamedQuerySpec.groovy # grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormStaticApi.groovy # grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/NamedCriteriaProxy.groovy # grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/NamedQuerySpec.groovy # grails-test-suite-uber/src/test/groovy/grails/compiler/DomainClassWithInnerClassUsingStaticCompilationSpec.groovy
|
I'm going to reopen this PR to get it fresh view. |
|
Replaced by #15568 |
🚨 TestLens detected 169 failed tests 🚨Here is what you can do:
Test Summary
🏷️ Commit: 732d30f Test Failures (first 5 of 183)AggregateMethodSpec (:grails-data-mongodb-core:test in CI - Groovy Joint Validation Build / build_grails)AssignedIdentifierSpec (:grails-data-mongodb-core:test in CI - Groovy Joint Validation Build / build_grails)AutoRunWithMultipleDataSourceSpec > runs app with a multiple datasource (:grails-data-hibernate7-dbmigration:integrationTest in CI - Groovy Joint Validation Build / build_grails)AutoRunWithMultipleDataSourceSpec > runs app with a multiple datasource (:grails-data-hibernate7-dbmigration:integrationTest in CI - Groovy Joint Validation Build / build_grails)AutoRunWithMultipleDataSourceSpec > runs app with a multiple datasource (:grails-data-hibernate7-dbmigration:integrationTest in CI - Groovy Joint Validation Build / build_grails)Muted TestsNote Checks are currently running using the configuration below. Select tests to mute in this pull request: 🔲 AggregateMethodSpec Reuse successful test results: 🔲 ♻️ Only rerun the tests that failed or were muted before Click the checkbox to trigger a rerun: 🔲 Rerun jobs Learn more about TestLens at testlens.app. |
I accidentally pushed to 8.0.x-hibernate7 instead of 8.0.x-hibernate7-bom. This caused #15510 to close as a result.
From that PR's todos:
Current Progress:
TODO:
I am opening this PR to track the status of Hibernate 7 merge to 8.x.
Some notes from previous meetings on the Hibernate 7 Upgrade: